Closed Bug 1941446 Opened 5 months ago Closed 3 months ago

Object literals with computed property names should allocate fixed slots

Categories

(Core :: JavaScript Engine, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
137 Branch
Tracking Status
firefox137 --- fixed

People

(Reporter: iain, Assigned: abdoatef.ab, Mentored)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

An object literal like {x:1, y: 2} generates a NewObject op that allocates an object of the required size and shape, then fills it in using InitProp. However, if it has a computed property key, like {x: 1, [key]: 2}, then we don't know the final shape. In that case, we fall back to emitting NewInit, which will call into NewPlainObject and allocate an object with AllocKind::Object0. We will allocate an object with no fixed slots, and then immediately allocate dynamic slots to hold the computed property.

We can't be completely precise here (consider let key = "x"; let o = { x: 1, [key]: 2}), but redefined properties should be rare enough that allocating a slot per defined property will not waste meaningful amounts of memory.

One possible implementation would be to add an operand to JSOp::NewInit indicating the expected number of properties, and reading it in NewObjectOperation. The NewObject baseline ICs work by calling NewObjectOperation and using the result as a template for future allocations, so if we can return a better sized object from NewObjectOperation, we shouldn't need to do any additional work in the jits.

This will make property access faster and reduce the likelihood that we allocate dynamic slots for these objects.

I'm not aware of this path being particularly hot in any benchmarks, but V8 did optimize it a while back.

I would like to work on this bug. Are there any resources available to help me get started with it?

Flags: needinfo?(mgaudet)

So, first and foremost make sure you can build and test SpiderMonkey.

The next thing to do would be to rummage through the parser and start to develop an understanding of the parse tree that gets generated. when you build the shell you get access to the parse builtin function which will dump the AST such that you can start to see how it looks:

js> parse("let x =  {[12]: 12}")
(StatementList [(LetDecl [(AssignExpr x
                                      (ObjectExpr [(PropertyDefinition (ComputedName 12)
                                                                       12)]))])])js> 

You'll also want to get familiar with bytecode and how to read and understand it. the dis function will help here:

js> function f() { let x =  {[12]: 12} }
js> dis(f)

{
  "file": "typein",
  "lineno": 2,
  "column": 11,
  "immutableFlags": [
    "IsFunction",
    "HasMappedArgsObj"
  ],
  "functionName": "f",
  "functionFlags": [
    "NORMAL_KIND",
    "BASESCRIPT",
    "CONSTRUCTOR"
  ]
}
loc   line  op
----- ----  --
main:
00000:   2  Uninitialized               # UNINITIALIZED
00001:   2  InitLexical 0               # UNINITIALIZED
00005:   2  Pop                         # 
00006:   2  NewInit                     # OBJ
00007:   2  Int8 12                     # OBJ 12
00009:   2  ToPropertyKey               # OBJ TOPROPERTYKEY(12)
00010:   2  Int8 12                     # OBJ TOPROPERTYKEY(12) 12
00012:   2  InitElem                    # OBJ
00013:   2  InitLexical 0               # OBJ
00017:   2  Pop                         # 
00018:   2  DebugLeaveLexicalEnv        # 
00019:   2  RetRval                     # 

Source notes:
 ofs line column    pc  delta desc             args
---- ---- ------ ----- ------ ---------------- ------
  0:    2     11     0 [   0] colspan          colspan 3
  2:    2     14     6 [   6] colspan          colspan 11
  4:    2     25     6 [   0] breakpoint-step-sep
  5:    2     25     7 [   1] colspan          colspan 1
  7:    2     26    19 [  12] colspan          colspan 10
  9:    2     36    19 [   0] breakpoint      

Exception table:
kind               stack    start      end

Scope notes:
   index   parent    start      end
       1   (none)        0       19

GC things:
   index   type       value
       0   Scope      function {} -> global
       1   Scope      function lexical {
                         0: let x (frame slot 0)
                      } -> function -> global

Next, we'll want to figure out how to estimate the computed property count. My intuition says something along the lines of what was done in Bug 1823811. In this case, when starting to parse an ObjectExpr we'll default the computed property count estimate to 0; then, when we encounter a nested ComputedName, we will need to update the count for the enclosing ObjectExpr.

After that, as Iain suggested, we'll want a parameter for NewInit that suggests an object size class.

Some other pointers;

Flags: needinfo?(mgaudet)

In this case, when starting to parse an ObjectExpr we'll default the computed property count estimate to 0; then, when we encounter a nested ComputedName, we will need to update the count for the enclosing ObjectExpr.

Note that we probably want to count all properties, not just computed properties. If we have {x: 1, y: 2, [z]: 3}, then we want to make sure we have enough room for at least 3 properties.

In this case, when starting to parse an ObjectExpr we'll default the computed property count estimate to 0; then, when we encounter a nested ComputedName, we will need to update the count for the enclosing ObjectExpr.

I followed your advice, but I noticed that EmitObject already has a propertyCount. Why can't I just use this propertyCount and emit it using emit2(propCount, JSOp::NewInt)? Then, I could add a propCount parameter to NewPlainObject and allocate the correct number of slots upfront.

Also, shouldn't the allocated slots have different sizes? Still, I don't know how I should proceed with this.

Flags: needinfo?(mgaudet)

Huh! fascinating.

Well, I would do a little validation that said property count is approximately correct, but after that, yes roughly your plan looks good (I think your call to emit2 seems backwards)

The allocated object will have a different count of slots depending on the estimate of the number of properties it will have. All slots are the same size (sizeof(Value)) .

You'll need to tweak the definition of NewInit and any consumers therein (ie. [baseline compiler,](https://searchfox.org/mozilla-central/source/js/src/jit/BaselineCodeGen.cpp#2980-2983, warp compiler etc). -- both of those look like they've backed by the NewObject inline cache, here. These seems to use a 'template object', obtained here which would be where you consume your estimate, in Baseline.

Warp looks a bit different, and I haven't traced fully.

My recommendation would be to write a patch for baseline, then tackle warp as a second patch.

Flags: needinfo?(mgaudet)
Assignee: nobody → abdoatef.ab
Status: NEW → ASSIGNED

i just wanted to check if i am going in the right direction

Flags: needinfo?(mgaudet)

Yep! Left review comments, but you're on the right road.

Flags: needinfo?(mgaudet)
Pushed by mgaudet@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/1c27909f60d6 Allocate fixed slots for object literals with computed property names. r=mgaudet
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → 137 Branch

Improved six-speed-object-literal-ext-es6 by 26.5%

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: